-
-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(rust, python): Decimal arithmetic #9123
Conversation
Sorry I've fallen out for a while, completely loaded with daytime work right :/ (But I'll be happy to participate in discussions at least.) I think the biggest quirk is this:
So:
|
// The division is done using the numbers without scale. | ||
// The dividend is scaled up to maintain precision after the | ||
// division |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The a * scale
will overflow very often if dealing with high-precision decimals. Basically what you're doing here implies that using scale greater than MAX_SCALE / 2
is not possible and will lead to overflow errors, I think.
Usually it's done in the 256-bit space (IIRC that's how arrow2 does it too).
// 222.222 --> 222222 | ||
// -------- ------- | ||
// 24691.308 <-- 24691308642 | ||
a * b / scale |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, a * b
will overflow too often. This is usually done with i256.
Re: precision, see very detailed discussion here – I don't think it should be used in any computations at all, and once you start doing any arithmetic work it can be simply discarded. The only thing that matters is Setting precision 'at the edges' should be sufficient (coming from python's decimal, you don't even have precision); so, reading and then immediately writing should preserve it. But if you start doing arithmetic stuff, I think it can be just ditched. You can always do a cast(Decimal(scale, precision)) at the end of the operation chain manually, or before writing, might you want to. |
There are more exceptions. The
Thanks, I will see if I can include that. Though, after a handful of multiplications we are out of sale range. How do people deal with that? Something like this:
Yes, now that I am looking into decimals, I am wondering if we should drop precision altogether. I don't see any benefit for it yet. |
Still to be decided if scale should adapt or not. @aldanor, do you have any ideas on this?